From 0f362cf30b3e336c537ed21d3cfb3c2ebd4bf3d7 Mon Sep 17 00:00:00 2001 From: Mark Simulacrum Date: Thu, 29 Mar 2018 12:19:19 +0200 Subject: [PATCH] Remove thread local, replacing with DeserializeSeed --- src/cargo/sources/registry/index.rs | 11 +- src/cargo/sources/registry/mod.rs | 175 +++++++++++++++++++++------- 2 files changed, 137 insertions(+), 49 deletions(-) diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 55da37612..74a687b76 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -2,12 +2,13 @@ use std::collections::HashMap; use std::path::Path; use std::str; +use serde::de::DeserializeSeed; use serde_json; use semver::Version; use core::dependency::Dependency; use core::{PackageId, SourceId, Summary}; -use sources::registry::{RegistryPackage, INDEX_LOCK}; +use sources::registry::{RegistryPackage, RegistryPackageDefault, INDEX_LOCK}; use sources::registry::RegistryData; use util::{internal, CargoResult, Config, Filesystem}; @@ -140,6 +141,7 @@ impl<'cfg> RegistryIndex<'cfg> { /// /// The returned boolean is whether or not the summary has been yanked. fn parse_registry_package(&mut self, line: &str) -> CargoResult<(Summary, bool)> { + let mut json_de = serde_json::Deserializer::from_str(line); let RegistryPackage { name, vers, @@ -148,12 +150,7 @@ impl<'cfg> RegistryIndex<'cfg> { features, yanked, links, - } = super::DEFAULT_ID.with(|slot| { - *slot.borrow_mut() = Some(self.source_id.clone()); - let res = serde_json::from_str::(line); - drop(slot.borrow_mut().take()); - res - })?; + } = RegistryPackageDefault(&self.source_id).deserialize(&mut json_de)?; let pkgid = PackageId::new(&name, &vers, &self.source_id)?; let summary = Summary::new(pkgid, deps.inner, features, links)?; let summary = summary.set_checksum(cksum.clone()); diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 949c44a22..95b0d11ac 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -159,7 +159,6 @@ //! ``` use std::borrow::Cow; -use std::cell::RefCell; use std::collections::BTreeMap; use std::fmt; use std::fs::File; @@ -213,18 +212,117 @@ pub struct RegistryConfig { pub api: Option, } -#[derive(Deserialize)] -struct RegistryPackage<'a> { +pub struct RegistryPackage<'a> { name: Cow<'a, str>, vers: Version, deps: DependencyList, features: BTreeMap>, cksum: String, yanked: Option, - #[serde(default)] links: Option, } +pub struct RegistryPackageDefault<'a>(pub &'a SourceId); + +impl<'de, 'a> de::DeserializeSeed<'de> for RegistryPackageDefault<'a> { + type Value = RegistryPackage<'de>; + + fn deserialize(self, deserializer: D) -> Result + where + D: de::Deserializer<'de>, + { + deserializer.deserialize_map(self) + } +} + +#[derive(Deserialize)] +#[serde(field_identifier, rename_all = "lowercase")] +enum Field { + Name, + Vers, + Deps, + Features, + Cksum, + Yanked, + Links, +} + +impl<'a, 'de> de::Visitor<'de> for RegistryPackageDefault<'a> { + type Value = RegistryPackage<'de>; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + write!(formatter, "struct RegistryPackage") + } + + fn visit_map(self, mut map: A) -> Result + where + A: de::MapAccess<'de>, + { + let mut name = None; + let mut vers = None; + let mut deps = None; + let mut features = None; + let mut cksum = None; + let mut yanked = None; + let mut links = None; + while let Some(key) = map.next_key()? { + match key { + Field::Name => { + if name.is_some() { + return Err(de::Error::duplicate_field("name")); + } + name = Some(map.next_value()?); + } + Field::Vers => { + if vers.is_some() { + return Err(de::Error::duplicate_field("vers")); + } + vers = Some(map.next_value()?); + } + Field::Deps => { + if deps.is_some() { + return Err(de::Error::duplicate_field("deps")); + } + deps = Some(map.next_value_seed(DependencyListDefault(self.0))?); + } + Field::Features => { + if features.is_some() { + return Err(de::Error::duplicate_field("features")); + } + features = Some(map.next_value()?); + } + Field::Cksum => { + if cksum.is_some() { + return Err(de::Error::duplicate_field("cksum")); + } + cksum = Some(map.next_value()?); + } + Field::Yanked => { + if yanked.is_some() { + return Err(de::Error::duplicate_field("yanked")); + } + yanked = Some(map.next_value()?); + } + Field::Links => { + if links.is_some() { + return Err(de::Error::duplicate_field("links")); + } + links = Some(map.next_value()?); + } + } + } + Ok(RegistryPackage { + name: name.ok_or_else(|| de::Error::missing_field("name"))?, + vers: vers.ok_or_else(|| de::Error::missing_field("vers"))?, + deps: deps.ok_or_else(|| de::Error::missing_field("deps"))?, + features: features.ok_or_else(|| de::Error::missing_field("features"))?, + cksum: cksum.ok_or_else(|| de::Error::missing_field("cksum"))?, + yanked: yanked.ok_or_else(|| de::Error::missing_field("yanked"))?, + links: links.unwrap_or(None), + }) + } +} + struct DependencyList { inner: Vec, } @@ -446,54 +544,47 @@ impl<'cfg> Source for RegistrySource<'cfg> { } } -// TODO: this is pretty unfortunate, ideally we'd use `DeserializeSeed` which -// is intended for "deserializing with context" but that means we couldn't -// use `#[derive(Deserialize)]` on `RegistryPackage` unfortunately. -// -// I'm told, however, that https://github.com/serde-rs/serde/pull/909 will solve -// all our problems here. Until that lands this thread local is just a -// workaround in the meantime. -// -// If you're reading this and find this thread local funny, check to see if that -// PR is merged. If it is then let's ditch this thread local! -thread_local!(static DEFAULT_ID: RefCell> = Default::default()); - -impl<'de> de::Deserialize<'de> for DependencyList { - fn deserialize(deserializer: D) -> Result +struct DependencyListDefault<'a>(&'a SourceId); + +impl<'a, 'de> de::DeserializeSeed<'de> for DependencyListDefault<'a> { + type Value = DependencyList; + fn deserialize(self, deserializer: D) -> Result where D: de::Deserializer<'de>, { - return deserializer.deserialize_seq(Visitor); - - struct Visitor; - - impl<'de> de::Visitor<'de> for Visitor { - type Value = DependencyList; + deserializer.deserialize_seq(self) + } +} - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - write!(formatter, "a list of dependencies") - } +impl<'de, 'a> de::Visitor<'de> for DependencyListDefault<'a> { + type Value = DependencyList; - fn visit_seq(self, mut seq: A) -> Result - where - A: de::SeqAccess<'de>, - { - let mut ret = Vec::new(); - if let Some(size) = seq.size_hint() { - ret.reserve(size); - } - while let Some(element) = seq.next_element::()? { - ret.push(parse_registry_dependency(element).map_err(|e| de::Error::custom(e))?); - } + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + write!(formatter, "a list of dependencies") + } - Ok(DependencyList { inner: ret }) - } + fn visit_seq(self, mut seq: A) -> Result + where + A: de::SeqAccess<'de>, + { + let mut ret = Vec::new(); + if let Some(size) = seq.size_hint() { + ret.reserve(size); } + while let Some(element) = seq.next_element::()? { + let dep = parse_registry_dependency(element, &self.0).map_err(de::Error::custom)?; + ret.push(dep); + } + + Ok(DependencyList { inner: ret }) } } /// Converts an encoded dependency in the registry to a cargo dependency -fn parse_registry_dependency(dep: RegistryDependency) -> CargoResult { +fn parse_registry_dependency( + dep: RegistryDependency, + default: &SourceId, +) -> CargoResult { let RegistryDependency { name, req, @@ -508,7 +599,7 @@ fn parse_registry_dependency(dep: RegistryDependency) -> CargoResult let id = if let Some(registry) = registry { SourceId::for_registry(®istry.to_url()?)? } else { - DEFAULT_ID.with(|id| id.borrow().as_ref().unwrap().clone()) + default.clone() }; let mut dep = Dependency::parse_no_deprecated(&name, Some(&req), &id)?; -- 2.30.2